-
Notifications
You must be signed in to change notification settings - Fork 7
Abstracting MathFlexOptions to EnergyBoundariesFlexOptions, integrating optimization into OptimizedFlexStrat
#1580
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Conversation
715bb5a to
10f59f6
Compare
# Conflicts: # src/test/scala/edu/ie3/simona/model/participant/PvModelSpec.scala # src/test/scala/edu/ie3/simona/model/participant/WecModelSpec.scala
danielfeismann
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some comments from my side, I'm happy to discuss them
src/main/scala/edu/ie3/simona/model/em/opt/PowerObjectiveFactory.scala
Outdated
Show resolved
Hide resolved
src/main/scala/edu/ie3/simona/ontology/messages/flex/EnergyBoundariesFlexOptions.scala
Show resolved
Hide resolved
| * @param sampleTime | ||
| * The amount of time between the steps. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would this be the unit to which you convert discrete time events? It might be helpful to provide some additional explanatory comments on this in general here or in the method header above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know what you mean with "unit"... It's just the amount of time between time steps. Time steps are defined in line 28-29 above. But maybe I can make the description more clear?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unit was the wrong term... interval would it have been, sorry.
So, just for my understanding: all discrete time events we have will be sampled to this interval, right? Perhaps we can add this in the method header (line 31) or line 34? E.g. This requires to convert the time series to fit into the set sampleTime. or similar?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The energy boundaries do not have to be sampled in this exact resolution, no. Although it's more efficient if that happens, of course.
src/test/scala/edu/ie3/simona/model/em/opt/OptimizedFlexStratSpec.scala
Outdated
Show resolved
Hide resolved
src/test/scala/edu/ie3/simona/model/em/opt/OptimizedFlexStratSpec.scala
Outdated
Show resolved
Hide resolved
src/test/scala/edu/ie3/simona/model/em/opt/OptimizedFlexStratSpec.scala
Outdated
Show resolved
Hide resolved
src/test/scala/edu/ie3/simona/model/em/opt/OptimizedFlexStratSpec.scala
Outdated
Show resolved
Hide resolved
Co-authored-by: Daniel Feismann <98817556+danielfeismann@users.noreply.github.com>
This PR introduces
EnergyBoundariesFlexOptions, which separates the flex options from the optimization logic.Also fixes an erroneous soft constraint penalty calculation.
Resolves #1572